Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

minfo.d is no longer throwing Errors #2795

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

WalterBright
Copy link
Member

assert() is better.

@dlang-bot
Copy link
Contributor

dlang-bot commented Sep 13, 2019

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + druntime#2795"

@WalterBright
Copy link
Member Author

uncaught exception
object.Exception@src/rt_trap_exceptions_drt.d(4): foo

I have no idea where that is coming from :-(

@thewilsonator
Copy link
Contributor

@thewilsonator
Copy link
Contributor

@WalterBright
Copy link
Member Author

@thewilsonator thanks!

@@ -368,7 +368,8 @@ struct ModuleGroup

string errmsg = "";
buildCycleMessage(idx, midx, (string x) {errmsg ~= x;});
throw new Error(errmsg, __FILE__, __LINE__);
assert(0, errmsg);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right though ? That's an user-facing message, not a programming error

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assert gives a user-facing message. Besides, being unable to run the constructors is a programming error.

@WalterBright
Copy link
Member Author

buildkite fails with the completely unhelpful:

0.000s PASS debug64 etc.linux.memoryerror
--
  | done
  | make[1]: Leaving directory '/var/lib/buildkite-agent/builds/ci-agent-ebaba67d-b3b1-4059-a201-c882003602cf-8/dlang/druntime/druntime'
  | 🚨 Error: The command exited with status 2

@WalterBright
Copy link
Member Author

The autotester logs show no failures, but list the tests as failing.

@Geod24
Copy link
Member

Geod24 commented Sep 14, 2019

The autotester logs show no failures, but list the tests as failing.

It does, but since the tests are run in parallel, the output is intertwined.

Testing cycle_abort
timelimit -t 10 ./generated/osx/release/64/test_cycles --DRT-oncycle=abort > generated/osx/release/64/cycle_abort.done 2>&1; test $? -eq 1
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.rpcnsip
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.rpcnterr
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.schannel
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.secext
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.security
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.servprov
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.setupapi
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.shellapi
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.shldisp
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.shlguid
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.shlobj
timelimit -t 10 generated/osx/debug/64/unittest/test_runner core.sys.windows.shlwapi
make[2]: *** [generated/osx/release/64/cycle_abort.done] Error 1
make[1]: *** [test/cycles/.run] Error 2
make: *** [unittest-release] Error 2
make: *** Waiting for unfinished jobs....

Test is checking for a specific return code, and I believe it changes from 1 to 4 with the switch to assert. See

$(ROOT)/cycle_abort.done: RETCODE=1

Copy link
Member

@schveiguy schveiguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does assert(0) print a proper message in release mode? It's critical that this doesn't just exit with a signal. Remeber that druntime is compiled in release mode.

If you want to print messages and then assert(0), that's OK, but these have to produce messages, or nobody will know what is going on.

Testing on run.dlang.io, assert(0, "message") does not print anything in release mode.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants